Test selecting disk by uuid+label #2877

Itxaka · 2024-09-23T08:18:35Z

What this PR does / why we need it:
Adds tests for the new functionality at kairos-io/kairos-agent#552

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1879

Will get green once the PR is merged, new version released and new framework created and added as part of this PR

Signed-off-by: Itxaka <[email protected]>

Itxaka · 2024-09-23T09:55:25Z

It passed when building with the new branch that brings this change, now that its removed, the test should fail

Signed-off-by: Itxaka <[email protected]>

Itxaka · 2024-09-24T13:16:09Z

now its pointing to master framework, so it should pass

EDIT: Now it has the final fix! should pass?

jimmykarily · 2024-09-25T09:09:01Z

tests/install_target_test.go

+				stateContains(vm, "system.os.name", "alpine", "opensuse", "ubuntu", "debian")
+				stateContains(vm, "kairos.flavor", "alpine", "opensuse", "ubuntu", "debian")
+			})
+		})


I don't see any check that the correct disk was actually used. We only check that the command didn't fail and that installation occurred. I guess, since we only have one disk, it was indeed selected but would be better if we had 2 disks and we made sure the correct one was selected, to avoid for example the flag being completely ignored and the default disk (bigger one?) gets selected instead.

But it can be tricky:

kairos/tests/custom_partitioning_test.go

Lines 68 to 76 in 15f342f

It("installs on the pre-configured disks", func() {

Expect(installError).ToNot(HaveOccurred(), installationOutput)

By("Rebooting into live CD again")

// In qemu it's tricky to boot the second disk. In multiple disk scenarios,

// setting "-boot=cd" will make qemu try to boot from the first disk and

// then from the cdrom.

// We want to make sure that kairos-agent selected the second disk so we

// simply let it boot from the cdrom again. Hopefully if the installation

// failed, we would see the error from the manual-install command.

I don't have a strong opinion, I'll let you decide.

Yes indeed, that's a good point. I am letting it work as is because if you pass a wrong or nonexistent disk, the install fails but I'll have a look at peg to see if we can pass multiple disks to the VM, that would make it more robust

spectrocloud/peg@c12a736

I introduced it for this exact reason. It's the boot order that is hard to do in qemu I think (see the link above).

but boot order should not matter no? It will try first all the disks and then livecd? Or it just tries the first disk only?

I don't recall to be honest. Maybe back then I was no aware of how to setup boot order using bootindex in qemu. We need to look at it again.

You are rigth, it doesnt try to boot from the second one somehow 😭

I could not make it work, maybe we should introduce a config in peg to choose the boot device or something, would be difficult to have it working for all, but at least we could make it work for qemu

Maybe we can add a default bootindex to all disks in qemu in peg, as in: first check disks (/dev/vda, /dev/vdb, etc) then cdrom.
This should allow us to try to target the second disk and see if it worked (since vda will be skipped as not being bootable).

Signed-off-by: Itxaka <[email protected]>

Itxaka · 2024-09-25T14:30:14Z

Now passing :D

jimmykarily · 2024-09-25T14:34:23Z

tests/install_target_test.go

+		Expect(err).ToNot(HaveOccurred())
+
+		vm.EventuallyConnects(1200)
+		// Format the second disk so it gets an uuid and label


/dev/vda would be the "first" disk no?

Signed-off-by: Itxaka <[email protected]>

Itxaka · 2024-09-25T14:53:08Z

rebase and merge once #2891 gets in with the new framework already

Itxaka added 3 commits September 23, 2024 10:18

[WIP] Test selecting disk by uuid

35eb689

Signed-off-by: Itxaka <[email protected]>

Split the device target tests

8d9de47

Signed-off-by: Itxaka <[email protected]>

Add test for targets

8c0cd12

Signed-off-by: Itxaka <[email protected]>

Itxaka force-pushed the test_uuid_disk branch from b64c2cf to 8c0cd12 Compare September 23, 2024 09:54

Itxaka changed the title ~~[WIP] Test selecting disk by uuid~~ [WIP] Test selecting disk by uuid+label Sep 23, 2024

Fix tests :D

ee85536

Signed-off-by: Itxaka <[email protected]>

Itxaka force-pushed the test_uuid_disk branch from d135e0b to ee85536 Compare September 23, 2024 10:27

Itxaka added 2 commits September 23, 2024 12:27

Merge branch 'master' into test_uuid_disk

36e9314

fix test

c98064e

Signed-off-by: Itxaka <[email protected]>

Itxaka force-pushed the test_uuid_disk branch from c29e1e0 to 147b743 Compare September 23, 2024 13:25

Fix test

cddc008

Signed-off-by: Itxaka <[email protected]>

Itxaka force-pushed the test_uuid_disk branch from 147b743 to cddc008 Compare September 23, 2024 14:04

Itxaka requested a review from a team September 23, 2024 15:11

Itxaka changed the title ~~[WIP] Test selecting disk by uuid+label~~ Test selecting disk by uuid+label Sep 23, 2024

Itxaka marked this pull request as ready for review September 23, 2024 15:12

Itxaka added 2 commits September 24, 2024 15:15

Merge branch 'master' into test_uuid_disk

644e784

Point to master framework

20c6b90

Signed-off-by: Itxaka <[email protected]>

Merge branch 'master' into test_uuid_disk

54a5520

jimmykarily reviewed Sep 25, 2024

View reviewed changes

Improve the test...maybe?

4b1a5c7

Signed-off-by: Itxaka <[email protected]>

Itxaka requested review from jimmykarily and a team September 25, 2024 14:30

jimmykarily reviewed Sep 25, 2024

View reviewed changes

Set proper framework

1fd30b3

Signed-off-by: Itxaka <[email protected]>

Itxaka force-pushed the test_uuid_disk branch from 85051e0 to 1fd30b3 Compare September 25, 2024 14:37

jimmykarily approved these changes Sep 25, 2024

View reviewed changes

Merge branch 'master' into test_uuid_disk

5379581

Itxaka merged commit c4e8b16 into master Sep 25, 2024
8 of 16 checks passed

Itxaka deleted the test_uuid_disk branch September 25, 2024 18:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test selecting disk by uuid+label #2877

Test selecting disk by uuid+label #2877

Itxaka commented Sep 23, 2024 •

edited

Loading

Itxaka commented Sep 23, 2024

Itxaka commented Sep 24, 2024 •

edited

Loading

jimmykarily Sep 25, 2024

Itxaka Sep 25, 2024

jimmykarily Sep 25, 2024

jimmykarily Sep 25, 2024

Itxaka Sep 25, 2024

jimmykarily Sep 25, 2024 •

edited

Loading

Itxaka Sep 25, 2024

Itxaka Sep 25, 2024

jimmykarily Sep 25, 2024

Itxaka commented Sep 25, 2024

jimmykarily Sep 25, 2024

Itxaka Sep 25, 2024

Itxaka commented Sep 25, 2024

	It("installs on the pre-configured disks", func() {
	Expect(installError).ToNot(HaveOccurred(), installationOutput)
	By("Rebooting into live CD again")
	// In qemu it's tricky to boot the second disk. In multiple disk scenarios,
	// setting "-boot=cd" will make qemu try to boot from the first disk and
	// then from the cdrom.
	// We want to make sure that kairos-agent selected the second disk so we
	// simply let it boot from the cdrom again. Hopefully if the installation
	// failed, we would see the error from the manual-install command.

Test selecting disk by uuid+label #2877

Test selecting disk by uuid+label #2877

Conversation

Itxaka commented Sep 23, 2024 • edited Loading

Itxaka commented Sep 23, 2024

Itxaka commented Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimmykarily Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Itxaka commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Itxaka commented Sep 25, 2024

Itxaka commented Sep 23, 2024 •

edited

Loading

Itxaka commented Sep 24, 2024 •

edited

Loading

jimmykarily Sep 25, 2024 •

edited

Loading